Skip to content

Conversation

@lokeshj1703
Copy link
Contributor

What changes were proposed in this pull request?

TestOzoneRpcClientAbstract.testListVolume is disabled due to intermittent issues.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-3062

How was this patch tested?

Fixes UT

@lokeshj1703 lokeshj1703 requested a review from adoroszlai July 10, 2020 12:46
@lokeshj1703 lokeshj1703 self-assigned this Jul 10, 2020
@lokeshj1703
Copy link
Contributor Author

Ran the test 20 times locally. There were no failures.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lokeshj1703 for working on this. The change itself looks good. However, I think we have some flakiness due to environmental or Ratis issues, which happens both in integration and acceptance tests (see my comment in HDDS-3907 about the latter). I think enabling further integration tests without addressing the root cause will trigger more frequent intermittent failures. We have observed this with other recently enabled integration tests (TestWatchForCommit, etc.), which were relatively stable in test runs (20x passed) but still fail intermittently in CI.

@lokeshj1703
Copy link
Contributor Author

@adoroszlai Thanks for reviewing the PR! I agree. The appendEntries timeout issue will affect all the ratis client related tests that is why we see failures in TestWatchForCommit, TestBlockDeletion etc. But I think we can still enable tests which do not use ratis client or the tests not involved in write path. The test enabled in the PR creates a bunch of volumes and then checks if listVolumes works well. What do you think?

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think we can still enable tests which do not use ratis client or the tests not involved in write path. The test enabled in the PR creates a bunch of volumes and then checks if listVolumes works well. What do you think?

Good point. This one seems safe to merge after a green run, since only one mini cluster is started per class.

@adoroszlai adoroszlai merged commit 8bb452f into apache:master Jul 14, 2020
@adoroszlai
Copy link
Contributor

Thanks @lokeshj1703 for the fix.

ChenSammi pushed a commit that referenced this pull request Jul 22, 2020
rakeshadr pushed a commit to rakeshadr/hadoop-ozone that referenced this pull request Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants